Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added normal mapping example #305

Merged
merged 18 commits into from
Oct 27, 2023
Merged

Conversation

cmhhelgeson
Copy link
Contributor

An example of normal mapping that allows the user to adjust the lighting of the scene, the depth layers, and the currently displayed combination of diffuse, normal, and height maps (Spiral, Toybox, Bark).

Current issues to be addressed:
Parallax Occlusion Mapping (Possibly in a later PR)
Weird Bark Height Map Behavior
Creating Generalized Way of Loading Images in Assets Folder from Helper Functions Rather Than Public
Removing unused types, interfaces, and helper functions per usual.

@cmhhelgeson
Copy link
Contributor Author

Ready for review. Contains some of the same utils as used in the bitonicSort shader, which can either be inlined, kept, or moved to a separate folder depending on the maintainers' preferences.

@cmhhelgeson cmhhelgeson marked this pull request as ready for review October 19, 2023 22:55
@cmhhelgeson
Copy link
Contributor Author

Adding video to demonstrate intended functionality.

Normal.Mapping.-.WebGPU.Samples.-.Google.Chrome.2023-10-25.17-03-15.mp4

@austinEng austinEng requested a review from greggman October 26, 2023 00:13
@austinEng
Copy link
Collaborator

@greggman could you help review this PR?

Copy link
Collaborator

@greggman greggman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I don't know how opinionated to be on this review. Please excuse me if I'm being too strict. I'm just going with my own thoughts. I'm also unfamiliar with the other samples. They may have similar issues.

I ran it locally and ran into the following issues

  • The cube is too far away to see any details

To fix I set the camera to this

    cameraPosX: 0.0,
    cameraPosY: 0.8,   // was 0
    cameraPosZ: -1.4,  // was -2.4

I removed the x rotation from the cube since just moving the camera above the cube seemed less surprising

    // mat4.rotateX(modelMatrix, 10, modelMatrix);

and updated the perspective to 0.1 instead of 1 for near

  const projectionMatrix = mat4.perspective(
    (2 * Math.PI) / 5,
    aspect,
    0.1,   // was 1
    10.0   // was 100
  ) as Float32Array;
  • The view matrix seemed wrong. I changed to this
  function getViewMatrix() {
    return mat4.lookAt(
      [settings.cameraPosX, settings.cameraPosY, settings.cameraPosZ],
      [0, 0, 0],
      [0, 1, 0]
    );
  }
  • The light seemed wrong.

If the cube is at 0,0,0 and the camera is -Z then a light at

    lightPosX: 1.7,
    lightPosY: -0.7,
    lightPosZ: 1.9,

Would be behind the cube (camera is -Z, light is +Z), to right (X is positive), and under the cube (Y is negative).

Changed to

    lightPosX: 1.7,
    lightPosY: 0.7,   // above the cube
    lightPosZ: -1.9,  // same side as camera
  • Then, I couldn't see any difference in rendering between these "Bump Mode"s

    | 'Normal Texture'
    | 'Depth Texture'
    | 'Normal Map'
    | 'Parallax Scale'
    | 'Steep Parallax';

To make sure I wasn't crazy I added an 'animate' GUI setting that stops the rotation. To do that I added a clock

  const settings: GUISettings = {
    ...
    animate: true,
    ...
  };

  ...

  gui.add(settings, 'animate');

  ...

  let then = 0;
  let time = 0;
  function frame(now) {
    if (!pageState.active) return;

    const elapsedTime = settings.animate ? (then - now) * 0.001 : 0;
    time += elapsedTime;
    then = now;

    // Write to normal map shader
    const viewMatrix = getViewMatrix();
    const modelMatrix = getModelMatrix(time);

and updated getModelMatrix to

  function getModelMatrix(time: number) {
    const modelMatrix = mat4.create();
    mat4.identitåy(modelMatrix);
    mat4.rotateY(modelMatrix, time * -0.5, modelMatrix);
    return modelMatrix;
  }

Once I had that in I tried stopping the animation and switching the bump mode. There is no difference in rendering between any of them except for "Diffuse Texture"

Screen.Recording.2023-10-26.at.18.11.40.mov
  • I'm not sure why all the casts like this
    // Write to normal map shader
    const viewMatrixTemp = getViewMatrix();
    const viewMatrix = viewMatrixTemp as Float32Array;

    const modelMatrixTemp = getModelMatrix();
    const modelMatrix = modelMatrixTemp as Float32Array;

I changed to

    // Write to normal map shader
    const viewMatrix = getViewMatrix();
    const modelMatrix = getModelMatrix(time);

And I don't get any TypeScript warnings 🤷‍♂️

Here's the changes I made mentioned above

greggman@db27f54

src/components/SampleLayout.module.css Outdated Show resolved Hide resolved
src/meshes/box.ts Outdated Show resolved Hide resolved
src/meshes/box.ts Outdated Show resolved Hide resolved
src/sample/normalMap/normalMap.wgsl Outdated Show resolved Hide resolved
depthSample: f32,
depthScale: f32,
) -> vec2f {
if (mapInfo.mappingType == 4) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are uber shaders generally considered bad practice? Would this be better with a separate shader for each mode? I'd think we don't want people copying this example if it's not considered a good thing to do. But I might wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more understandable and performant than having to manage multiple pipelines, especially when different types of depth mapping are still using the same code, but I defer to your expertise.

src/sample/normalMap/utils.ts Outdated Show resolved Hide resolved
src/sample/normalMap/utils.ts Outdated Show resolved Hide resolved
src/meshes/box.ts Outdated Show resolved Hide resolved
src/sample/normalMap/main.ts Outdated Show resolved Hide resolved
src/sample/normalMap/main.ts Outdated Show resolved Hide resolved
@cmhhelgeson
Copy link
Contributor Author

I implemented most of the suggestions (although the perspective of my camera doesn't seem to match yours). However, I left in the model rotation, since I think it more effectively demonstrates how the perceptual depth changes as the surface moves in and out of light, and I wasn't sure what was meant by surprising (too visually distracting, unexpected?).

@cmhhelgeson
Copy link
Contributor Author

Also, I was able to fix the issue with the map mode not changing by remembering to write that value as a u32 rather than a f32.

@greggman greggman enabled auto-merge (squash) October 27, 2023 04:01
Copy link
Collaborator

@greggman greggman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@greggman greggman merged commit 7349a25 into webgpu:main Oct 27, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants